Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds handling for InvalidChunk exceptions in the Bigtable accelerator fallback mechanism. When an InvalidChunk is encountered, the breaker is tripped, a warning is logged, and the client falls back to the native Bigtable client. Unit tests have been added to verify this behavior. I have no feedback to provide as the implementation is clean and well-tested.
When an accelerator ReadRows stream yields a malformed chunk (such as an empty row key), merge_rows raises InvalidChunk. Catch InvalidChunk in handle_accelerator_error to trip the breaker and fall back to the native GAPIC client.
e3b84f8 to
2d1dd8e
Compare
sushanb
left a comment
sushanb
left a comment
There was a problem hiding this comment.
The InvalidChunk breaker + _AcceleratorFallback handling for read_row looks good, however holding off on approval due to a branch/CI blocker:
-
Blocker — Base branch targets closed/unmerged PR #18930 (
fix-merge-rows-empty-family) and includes its abandoned commit:fix-merge-rows-empty-family(#18930) was closed without merging, so this PR's branch (2d1dd8ecf5) still carries the unmerged commit4dbe3522eb(fix(bigtable): check family_name field presence in merge_rows), and merging as-is would merge intofix-merge-rows-empty-familyrather thanbigtable-accel.- Because the base branch is
fix-merge-rows-empty-family, none of the GitHub Actions unit test / lint / mypy checks ran on this PR (only 3 bot checks ran vs. 16 checks onbigtable-accel).
Please rebase onto
bigtable-accel(dropping4dbe3522eb) and retarget the base branch tobigtable-accelso the full CI suite runs.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕